AMM-1456 : JwtToken skiped if it not present from mobile #76
Conversation
WalkthroughThe JWT validation filter was refactored to sequentially check JWT tokens from cookies first, then headers, instead of choosing one source. If neither token is valid, the filter now allows requests from mobile clients with an "Authorization" header to proceed, otherwise returning a 401 error. A helper method for mobile client detection was added. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant JwtUserIdValidationFilter
participant Application
Client->>JwtUserIdValidationFilter: Send request (with cookies/headers)
alt Cookie JWT token is valid
JwtUserIdValidationFilter->>Application: Proceed with request
else Header JWT token is valid
JwtUserIdValidationFilter->>Application: Proceed with request
else Mobile client with Authorization header
JwtUserIdValidationFilter->>Application: Proceed with request
else
JwtUserIdValidationFilter-->>Client: Respond 401 Unauthorized
end
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/main/java/com/iemr/admin/utils/JwtUserIdValidationFilter.java (1)
109-119: 🛠️ Refactor suggestionCookie name inconsistency with header name.
There's a case sensitivity issue between cookie name "Jwttoken" (line 113) and header name "JwtToken" (line 69).
Use consistent naming across both cookie and header checks to prevent authentication failures:
if (cookie.getName().equals("Jwttoken")) {Should match the header name used in the validation logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
src/main/java/com/iemr/admin/utils/JwtUserIdValidationFilter.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java)
🔇 Additional comments (3)
src/main/java/com/iemr/admin/utils/JwtUserIdValidationFilter.java (3)
72-78: Improved JWT validation flow.The sequential validation first checking cookies then headers is a good approach. This provides better flow control and ensures the request only proceeds when a valid token is found.
80-86: Clear and efficient validation logic.The header token validation is well-structured with appropriate logging. The early return pattern after successful validation improves code readability and efficiency.
95-96: Improved error handling.Clear error messaging and appropriate status code for unauthorized access. The additional logging helps with troubleshooting authentication issues.
| private boolean isMobileClient(String userAgent) { | ||
| if (userAgent == null) | ||
| return false; | ||
| userAgent = userAgent.toLowerCase(); | ||
| return userAgent.contains("okhttp"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Mobile client detection is too simplistic.
The mobile client detection relies solely on checking for "okhttp" in the User-Agent string. This is fragile and can be easily circumvented.
Consider:
- Using a more comprehensive list of mobile client identifiers
- Implementing additional verification methods beyond User-Agent
- Creating a configurable list of trusted mobile clients
private boolean isMobileClient(String userAgent) {
if (userAgent == null)
return false;
userAgent = userAgent.toLowerCase();
- return userAgent.contains("okhttp");
+ // Check for common mobile client indicators
+ return userAgent.contains("okhttp") ||
+ userAgent.contains("android") ||
+ userAgent.contains("iphone") ||
+ userAgent.contains("mobile");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private boolean isMobileClient(String userAgent) { | |
| if (userAgent == null) | |
| return false; | |
| userAgent = userAgent.toLowerCase(); | |
| return userAgent.contains("okhttp"); | |
| } | |
| private boolean isMobileClient(String userAgent) { | |
| if (userAgent == null) | |
| return false; | |
| userAgent = userAgent.toLowerCase(); | |
| // Check for common mobile client indicators | |
| return userAgent.contains("okhttp") || | |
| userAgent.contains("android") || | |
| userAgent.contains("iphone") || | |
| userAgent.contains("mobile"); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/iemr/admin/utils/JwtUserIdValidationFilter.java around
lines 103 to 108, the current mobile client detection only checks if the
User-Agent contains "okhttp", which is too simplistic and unreliable. To fix
this, replace the hardcoded check with a configurable list of trusted mobile
client identifiers and update the method to check if the User-Agent matches any
entry in this list. Additionally, consider implementing extra verification
methods beyond User-Agent to improve detection accuracy.
| String jwtFromCookie = getJwtTokenFromCookies(request); | ||
| String jwtFromHeader = request.getHeader("JwtToken"); | ||
| String authHeader = request.getHeader("Authorization"); | ||
|
|
There was a problem hiding this comment.
Inconsistent JWT token header naming.
There's an inconsistency between the JWT token header name used in initial logging (line 52: "Jwttoken") and the one used for actual validation (line 69: "JwtToken"). This case difference could cause the header check to fail if clients are using the case shown in the logging statement.
- String jwtFromHeader = request.getHeader("JwtToken");
+ String jwtFromHeader = request.getHeader("Jwttoken");Also, consider using constants for header names to maintain consistency throughout the code.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| String jwtFromCookie = getJwtTokenFromCookies(request); | |
| String jwtFromHeader = request.getHeader("JwtToken"); | |
| String authHeader = request.getHeader("Authorization"); | |
| String jwtFromCookie = getJwtTokenFromCookies(request); | |
| - String jwtFromHeader = request.getHeader("JwtToken"); | |
| + String jwtFromHeader = request.getHeader("Jwttoken"); | |
| String authHeader = request.getHeader("Authorization"); |
🤖 Prompt for AI Agents
In src/main/java/com/iemr/admin/utils/JwtUserIdValidationFilter.java around
lines 52 and 69, the JWT token header name is inconsistently cased ("Jwttoken"
vs "JwtToken"), which can cause header retrieval failures. Standardize the
header name casing across all usages to be identical. Additionally, define a
constant for the JWT token header name and replace all hardcoded instances with
this constant to ensure consistency and easier maintenance.
| String userAgent = request.getHeader("User-Agent"); | ||
| logger.info("User-Agent: " + userAgent); | ||
|
|
||
| if (isValid) { | ||
| // If token is valid, allow the request to proceed | ||
| if (userAgent != null && isMobileClient(userAgent) && authHeader != null) { | ||
| filterChain.doFilter(servletRequest, servletResponse); | ||
| } else { | ||
| response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Invalid JWT token"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Security concern with mobile client bypass.
Allowing mobile clients to bypass JWT validation based solely on User-Agent and presence of an Authorization header creates a security vulnerability. The User-Agent can be easily spoofed.
Consider implementing a more robust authentication mechanism for mobile clients. At minimum, verify that the Authorization header contains a valid token:
🏁 Script executed:
#!/bin/bash
# Check if there's any validation for the Authorization header elsewhere in the codebase
rg -A 3 "Authorization.*header" --type javaLength of output: 45
🏁 Script executed:
#!/bin/bash
# Locate where the Authorization header is retrieved
rg -n 'getHeader.*"Authorization"' -A 3 --type java
# Show all occurrences of the local variable authHeader for context
rg -n 'authHeader' -A 5 --type javaLength of output: 4717
Ensure JWT validation for mobile clients, not just header presence
The current logic in JwtUserIdValidationFilter.java (around lines 87–93) skips any real token check for mobile UAs:
String userAgent = request.getHeader("User-Agent");
if (userAgent != null && isMobileClient(userAgent) && authHeader != null) {
filterChain.doFilter(servletRequest, servletResponse);
return;
}User-Agent values are trivial to spoof—allowing any non-null Authorization header effectively opens a backdoor. You should validate the token in the header just as you do for the cookie:
• File: src/main/java/com/iemr/admin/utils/JwtUserIdValidationFilter.java
• Lines: ~87–93
Suggested change:
- if (userAgent != null && isMobileClient(userAgent) && authHeader != null) {
- filterChain.doFilter(servletRequest, servletResponse);
- return;
- }
+ if (userAgent != null && isMobileClient(userAgent) && authHeader != null) {
+ logger.info("Validating JWT token from Authorization header for mobile client");
+ if (jwtAuthenticationUtil.validateUserIdAndJwtToken(authHeader)) {
+ filterChain.doFilter(servletRequest, servletResponse);
+ return;
+ }
+ logger.warn("Invalid JWT token in Authorization header for mobile client");
+ }This ensures mobile clients can’t bypass authentication by spoofing User-Agent.
🤖 Prompt for AI Agents
In src/main/java/com/iemr/admin/utils/JwtUserIdValidationFilter.java around
lines 87 to 93, the code currently bypasses JWT validation for mobile clients if
the User-Agent header indicates a mobile client and the Authorization header is
present, which is insecure due to easy User-Agent spoofing. To fix this, remove
the bypass condition and instead validate the JWT token in the Authorization
header for mobile clients just as you do for other clients, ensuring the token
is properly parsed and verified before allowing the request to proceed.
2 participants
📋 Description
JIRA ID: AMM-1456
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
✅ Type of Change
ℹ️ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit
Bug Fixes
Refactor